Skip to content

fix: retry rename on Windows transient file locks during install#10300

Merged
jdx merged 1 commit into
jdx:mainfrom
jhult:fix/retry-rename-on-windows
Jun 11, 2026
Merged

fix: retry rename on Windows transient file locks during install#10300
jdx merged 1 commit into
jdx:mainfrom
jhult:fix/retry-rename-on-windows

Conversation

@jhult

@jhult jhult commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Problem: file::rename on Windows fails with Access is denied (os error 5) during tool installation (see #4528). When installing core tools like node, mise extracts an archive to a temp directory and then fs::renames it into place. Antivirus (Windows Defender) can hold handles on recently-extracted files, causing the rename to fail transiently.

Fix: The file::rename function now retries up to 5 times on Windows when the error is ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32), with exponential backoff (50ms → 800ms). These are the same transient lock codes that the shims module already handles for locked .exe files .

The move_file function also benefits since it now calls the retry wrapper instead of raw fs::rename.

On non-Windows, behavior is unchanged - a single fs::rename call as before.

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability of file rename and move operations on Windows by automatically retrying transient access failures (e.g., antivirus/OS file locks) with exponential backoff up to 5 attempts.
    • Move operations now use the same Windows retry behavior before falling back to copy-and-remove for cross-device moves.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7a98f240-e2e3-4cdb-8486-9a9055877f92

📥 Commits

Reviewing files that changed from the base of the PR and between 58e5028 and 20c91d2.

📒 Files selected for processing (1)
  • src/file.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/file.rs

📝 Walkthrough

Walkthrough

Filesystem rename operations now use an internal do_rename helper that retries transient Windows rename errors (ERROR_ACCESS_DENIED and ERROR_SHARING_VIOLATION) with exponential backoff; rename and move_file call this helper before existing fallbacks.

Changes

Filesystem Rename Retry Mechanism

Layer / File(s) Summary
do_rename helper with Windows retry logic
src/file.rs
New platform-specific do_rename helper retries fs::rename on Windows for transient error codes 5 and 32 with exponential backoff (up to 5 attempts); on non-Windows it delegates to fs::rename once.
rename docs and integration
src/file.rs
rename documentation updated to describe Windows retry behavior; implementation now calls do_rename (preserving existing error wrapping) instead of calling fs::rename directly.
move_file uses do_rename
src/file.rs
move_file performs its initial rename via do_rename(from, to) so Windows retries apply before the cross-device copy+remove fallback.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant rename
  participant do_rename
  participant fs_rename
  Caller->>rename: rename(from, to)
  rename->>do_rename: do_rename(from, to)
  do_rename->>fs_rename: fs::rename(from, to)
  fs_rename-->>do_rename: Ok / Err(raw_os_error)
  do_rename->>do_rename: retry on 5 or 32 with exponential backoff
  do_rename-->>rename: Ok / Err(last_error)
  rename-->>Caller: Result (with existing error wrapping)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped and watched the file move slow,
On Windows locks that come and go,
Five gentle retries, backoff in tow,
A patient rename—quiet and low,
Files find their new homes, row by row.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding retry logic for Windows transient file locks during rename operations in the installation process.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR wraps fs::rename on Windows with a do_rename helper that retries up to 5 times on ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32), using exponential back-off (50 ms → 800 ms), to handle transient antivirus/OS file-handle locks during tool installation. Non-Windows behaviour is unchanged.

  • rename and move_file both now call do_rename so both benefit from the retry logic.
  • The back-off correctly skips the final sleep (guarded by attempt + 1 < MAX_ATTEMPTS) — the current code is correct on that point.

Confidence Score: 5/5

Safe to merge — the change is Windows-only and isolated to a single helper; non-Windows builds compile through the trivial one-liner branch.

The retry logic is small and self-contained: the back-off guard correctly avoids a pointless final sleep, non-retried errors are returned immediately without delay, and the unwrap() on last_err cannot panic. No behaviour changes on Linux or macOS.

No files require special attention.

Important Files Changed

Filename Overview
src/file.rs Adds Windows-only do_rename helper with 5-attempt exponential-backoff retry for transient lock errors (os error 5/32); wires both rename and move_file through it. Logic is correct and non-Windows path is unaffected.

Reviews (2): Last reviewed commit: "fix(install): retry rename on Windows tr..." | Re-trigger Greptile

Comment thread src/file.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/file.rs (1)

169-187: 💤 Low value

Consider adding trace logging for retry attempts.

The shims module logs when it encounters locked files (trace!("cannot delete locked shim...") in shims.rs). Adding similar logging here would aid debugging when installs are slow due to antivirus interference:

🔧 Suggested logging addition
 #[cfg(windows)]
 fn do_rename(from: &Path, to: &Path) -> std::io::Result<()> {
     const MAX_ATTEMPTS: u32 = 5;
     let mut last_err = None;
     for attempt in 0..MAX_ATTEMPTS {
         match fs::rename(from, to) {
             Ok(()) => return Ok(()),
             Err(e) if matches!(e.raw_os_error(), Some(5) | Some(32)) => {
                 // ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32):
                 // likely a transient lock from antivirus or the OS.
                 // Exponential backoff: 50ms, 100ms, 200ms, 400ms, 800ms
+                trace!(
+                    "rename {} -> {} failed with OS error {}, retrying ({}/{})",
+                    from.display(),
+                    to.display(),
+                    e.raw_os_error().unwrap_or(-1),
+                    attempt + 1,
+                    MAX_ATTEMPTS
+                );
                 last_err = Some(e);
                 std::thread::sleep(std::time::Duration::from_millis(50 * (1 << attempt)));
             }
             Err(e) => return Err(e),
         }
     }
     Err(last_err.unwrap())
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/file.rs` around lines 169 - 187, The do_rename function retries on
Windows but doesn't log retry attempts; add trace-level logging inside the
Err(e) branch that matches ERROR_ACCESS_DENIED/ERROR_SHARING_VIOLATION so each
retry reports the attempt number, the from and to paths, the error and the
backoff duration (use the same 50ms * (1 << attempt) calculation) using the
trace! macro (referencing do_rename, the loop's attempt variable, from and to
Path values, and the local e) and also add a final trace or debug log before
returning Err(last_err.unwrap()) to record the ultimate failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/file.rs`:
- Around line 169-187: The do_rename function retries on Windows but doesn't log
retry attempts; add trace-level logging inside the Err(e) branch that matches
ERROR_ACCESS_DENIED/ERROR_SHARING_VIOLATION so each retry reports the attempt
number, the from and to paths, the error and the backoff duration (use the same
50ms * (1 << attempt) calculation) using the trace! macro (referencing
do_rename, the loop's attempt variable, from and to Path values, and the local
e) and also add a final trace or debug log before returning
Err(last_err.unwrap()) to record the ultimate failure.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 65b61cd5-793f-410a-ae7a-38f97a130b68

📥 Commits

Reviewing files that changed from the base of the PR and between 3f2b853 and 9496b4b.

📒 Files selected for processing (1)
  • src/file.rs

@jhult jhult force-pushed the fix/retry-rename-on-windows branch 2 times, most recently from 58e5028 to 6705cd4 Compare June 11, 2026 00:26
When installing core tools like node on Windows, mise extracts archives to a temp directory then renames into place. Windows/antivirus can briefly lock files in the extracted directory, causing fs::rename to fail with ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32). This is the same class of transient lock that the shims module already handles for locked .exe files.

jdx#4528
@jhult jhult force-pushed the fix/retry-rename-on-windows branch from 6705cd4 to 20c91d2 Compare June 11, 2026 00:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/file.rs (1)

169-194: 💤 Low value

Consider adding trace logging on retry attempts.

When transient lock failures trigger retries, there's no visibility into the retry loop. Adding a trace! log (similar to the pattern in shims.rs at line 271) would help diagnose intermittent Windows installation failures.

🔧 Suggested trace logging
             Err(e) if matches!(e.raw_os_error(), Some(5) | Some(32)) => {
                 // ERROR_ACCESS_DENIED (5) or ERROR_SHARING_VIOLATION (32):
                 // likely a transient lock from antivirus or the OS.
                 // Exponential backoff: 50ms, 100ms, 200ms, 400ms, 800ms
                 last_err = Some(e);
                 if attempt + 1 < MAX_ATTEMPTS {
+                    trace!(
+                        "rename {} -> {} blocked by transient lock, retrying ({}/{})",
+                        from.display(),
+                        to.display(),
+                        attempt + 1,
+                        MAX_ATTEMPTS
+                    );
                     std::thread::sleep(std::time::Duration::from_millis(50 * (1 << attempt)));
                 }
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/file.rs` around lines 169 - 194, Add trace-level logging inside the
Windows retry branch of do_rename to surface each transient retry: in the Err(e)
if matches!(e.raw_os_error(), Some(5) | Some(32)) arm (inside fn do_rename),
call trace! with the attempt number, MAX_ATTEMPTS, the error (e), and the
from/to paths and computed backoff duration (follow the trace pattern used in
shims.rs around the retry on line 271) before sleeping so each retry and its
reason are visible for diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/file.rs`:
- Around line 169-194: Add trace-level logging inside the Windows retry branch
of do_rename to surface each transient retry: in the Err(e) if
matches!(e.raw_os_error(), Some(5) | Some(32)) arm (inside fn do_rename), call
trace! with the attempt number, MAX_ATTEMPTS, the error (e), and the from/to
paths and computed backoff duration (follow the trace pattern used in shims.rs
around the retry on line 271) before sleeping so each retry and its reason are
visible for diagnostics.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d58efaa-22a8-4dce-afba-b103cbcf8e6d

📥 Commits

Reviewing files that changed from the base of the PR and between 9496b4b and 58e5028.

📒 Files selected for processing (1)
  • src/file.rs

@jdx jdx enabled auto-merge (squash) June 11, 2026 00:33
@jdx jdx merged commit ecc7213 into jdx:main Jun 11, 2026
33 checks passed
@jhult jhult deleted the fix/retry-rename-on-windows branch June 11, 2026 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants